Skip to content

fix(sdk,desktop,cli): tag reactions with the target author (NIP-25 p tag)#2634

Open
shani-singh1 wants to merge 2 commits into
block:mainfrom
shani-singh1:fix/nip25-reaction-p-tag
Open

fix(sdk,desktop,cli): tag reactions with the target author (NIP-25 p tag)#2634
shani-singh1 wants to merge 2 commits into
block:mainfrom
shani-singh1:fix/nip25-reaction-p-tag

Conversation

@shani-singh1

Copy link
Copy Markdown

Fixes #2568.

Problem

Every kind:7 Buzz emits carries only the target e tag:

{"kind":7,"tags":[["e","8f8c92f8…"]],"content":"👍", }

NIP-25 also requires the reacted-to author's p tag. That tag is not cosmetic — it is the only thing a notification feed can key on. Without it, {"kinds":[7],"#p":[<self>]} can never match, so a reaction to your message is invisible to every NIP-01 client, which is what @vitorpamplona hit from Amethyst.

It isn't only an interop gap. The relay already ships kind 7 as push-notifiable — PUSH_KINDS = &[7, 9, 1059, 40007, 46010] (crates/buzz-relay/src/handlers/push_lease.rs:15) — and the lease subscription shape is literally {"kinds":[7],"#p":[…]}. The push surface is built around a tag no Buzz client produces.

Four producers were affected: buzz-sdk, the desktop Tauri command, buzz-cli, and buzz-acp's 👀/💬 status reactions.

Solution

Thread the target author through every reaction producer.

buzz-sdkbuild_reaction / build_custom_emoji_reaction take target_author: nostr::PublicKey and emit ["p", <author>]. Required rather than Option, so a caller can't silently reintroduce the non-conformant form.

Both builders now set .allow_self_tagging(). This is the subtle part: reacting to your own message makes the p tag match the signer, and nostr 0.44 strips matching p tags by default. Without it, self-reactions would quietly regress to the old wire form. The repo already relies on this for NIP-IA (build_archive_identity_request); the reaction path now does too, pinned by a regression test.

Desktopadd_reaction takes target_pubkey from the caller. The timeline already resolved it via resolveEventAuthorPubkey, so this uses the displayed author, not the raw signer. For relay-signed agent messages those differ, and the displayed author is who should be notified. Passing it in also avoids a second relay round trip on a hot UI path.

buzz-cli — only ever gets an event id, so it resolves the author with a kindless ids:[…] lookup. That filter shape is explicitly exempt from the #p read gate (req.rs:1061, "a kindless ids lookup is unaffected"). A target that doesn't resolve here would have been rejected by ingest anyway (invalid: reaction target event not found); failing early just gives a clearer message.

buzz-acp — the 👀/💬 indicators are reactions like any other, so they carry the tag too. The author is already on the batch event; react_working now takes (event_id, author) pairs. Removal is unchanged — a kind:5 delete keys off the reaction's own id, so ReactionGuard still only needs ids.

Tag order is e, p, emoji. Ingest resolves the reaction target from the last e tag (ingest.rs:2247), so adding a p tag cannot shift target resolution.

Why this is behaviour-neutral in-app

Worth stating explicitly, since "add a p tag" could plausibly mean "start waking agents on every 👍":

  • The desktop feed queries kinds [9, 40002, 1, 45001, 45003] (commands/messages.rs:72) — no kind 7.
  • Mobile's activity feed uses the same kind set (activity_provider.dart:33).
  • The ACP mention subscription defaults to [KIND_STREAM_MESSAGE, KIND_WORKFLOW_APPROVAL_REQUESTED, KIND_STREAM_REMINDER] (config.rs:1157) — no kind 7, so p-tagged reactions do not wake agents.
  • Kind 7 is not in P_GATED_KINDS, so no read path changes.

The only things that change are NIP-25 conformance on the wire and the relay's already-advertised push surface becoming reachable.

Validation

Toolchain note: this was built on Windows with the x86_64-pc-windows-gnu toolchain (no MSVC linker locally), so clippy is 1.97 rather than the pinned 1.95.

  • cargo test -p buzz-sdk --lib234 passed, 0 failed, including the new reaction_keeps_p_tag_when_reacting_to_own_message. I verified that test has teeth: with .allow_self_tagging() removed it fails with assertion failed: has_tag(&ev, "p", …), confirming the nostr 0.44 scrub is real and not theoretical.
  • cargo check -p buzz-sdk -p buzz-cli -p buzz-acp — clean.
  • cargo fmt -p buzz-sdk -p buzz-cli -p buzz-acp -- --check — clean.
  • cargo clippy -p buzz-sdk -p buzz-cli -p buzz-acp --all-targets -- -D warnings — the only findings are two pre-existing question_mark lints in crates/buzz-acp/src/queue.rs (313, 942), a file this PR does not touch; they come from the newer clippy, not from this change.
  • cargo check on desktop/src-tauri — clean (24 pre-existing dead-code warnings in managed_agents/, none from this PR).
  • pnpm typecheck and pnpm lint in desktop/ — both clean.
  • biome format on the six changed TS files — clean. (Checked against LF copies; my checkout is core.autocrlf=true, which makes biome report all 1600 files unformatted locally.)
  • Desktop JS unit tests could not run here: the loader crashes on Windows with ERR_UNSUPPORTED_ESM_URL_SCHEME … Received protocol 'c:' on a clean tree too. No test references add_reaction, so nothing in that suite covers this path either way.

The E2E mock (e2eBridge.ts) now emits the p tag as well, so inbox-reactions.spec.ts's "real add_reaction wire shape" assertion stays honest. It falls back to reading the author out of its own mock event store when a spec drives add_reaction directly without one, so the two existing specs that do that keep working unchanged.

Follow-up, not done here

NIP-25 also suggests a k tag carrying the target's kind, which helps clients filter reaction notifications. Every call site now has the target event in hand so it would be cheap, but it's a separate wire change and I kept this PR to the reported bug.

@shani-singh1
shani-singh1 requested a review from a team as a code owner July 23, 2026 22:12
…tag)

Every kind:7 Buzz emits carries only `["e", <target>]`. NIP-25 also requires
the reacted-to author's `p` tag, and that tag is the mechanism a notification
feed keys on: without it `{"kinds":[7],"#p":[<self>]}` can never match, so a
reaction to your message is invisible to every NIP-01 client — including
Amethyst, which is where this was reported from.

It is not only an interop gap. The relay already advertises kind 7 in
`PUSH_KINDS` (push_lease.rs) and the lease subscription shape is exactly
`{"kinds":[7],"#p":[...]}`, so the push surface is built for a tag no Buzz
client produces.

Thread the target author through every reaction producer:

- buzz-sdk `build_reaction` / `build_custom_emoji_reaction` take the author
  and emit the `p` tag. Required rather than optional so a caller cannot
  silently reintroduce the non-conformant form.
- Both builders set `.allow_self_tagging()`. nostr 0.44 strips a `p` tag
  matching the signer, which would have quietly dropped the tag whenever
  someone reacts to their own message; a regression test pins this.
- The desktop `add_reaction` command takes the target pubkey from the caller.
  The timeline already resolved it (`resolveEventAuthorPubkey`), so this uses
  the displayed author rather than the raw signer — for relay-signed agent
  messages those differ and the displayed author is who should be notified.
- buzz-cli resolves the author with an `ids:[...]` lookup, since it is only
  given an event id.
- buzz-acp's 👀/💬 status reactions carry the tag too; the author is already
  on the batch event.

No in-app behaviour changes: neither the desktop feed nor the mobile activity
filter includes kind 7, and the ACP mention subscription is kinds
[9, approval, reminder], so p-tagged reactions do not wake agents.

Fixes block#2568

Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
@shani-singh1
shani-singh1 force-pushed the fix/nip25-reaction-p-tag branch from fa91956 to 1dd3c7e Compare July 23, 2026 22:28
@dophsquare

Copy link
Copy Markdown

P0 triage — merge candidate. 🐝

Confirmed the interop gap: kind:7 emits carried only the e tag, but NIP-25 requires the reacted-to author's p tag, and it's the only thing a {"kinds":[7],"#p":[<self>]} notification filter can key on. Without it a reaction to your message is invisible to every NIP-01 client — exactly what @vitorpamplona hit from Amethyst. It's also a functional bug for Buzz itself since kind 7 ships in PUSH_KINDS.

The plumbing looks right: reaction_add now takes target_author: PublicKey for the p tag, and the 👀/💬 indicators are threaded through ReactionTarget { event_id, author } so they're conformant too rather than emitting non-conformant kind:7. Nice touch keeping ReactionGuard cleanup keyed on just the reaction's own id (kind:5 delete) — cleanup correctly doesn't need the author.

Ask before merge: any other kind:7 emit sites outside buzz-acp (SDK build_reaction callers in desktop/cli) that still omit the p tag? Title says sdk,desktop,cli so I assume covered — just want the negative confirmed. LGTM.

The Flutter client had the same gap as the desktop/CLI/SDK paths: its
`addReaction` emitted only `["e", target]` plus an optional NIP-30 emoji tag,
so mobile-authored reactions were invisible to any `{"kinds":[7],"#p":[self]}`
notification filter.

`ChannelActions.addReaction` now takes the target author and emits its `p`
tag. All four call sites already hold it — channel and thread rows and the
message-action sheet pass `TimelineMessage.pubkey`, and Pulse threads
`UserNote.pubkey` through `toggleNoteUpvote`.

Unlike the Rust builders this needs no self-tagging opt-out: the Dart signer
passes tags through verbatim, so a self-reaction keeps its `p` tag.

Found while confirming there were no remaining kind:7 emit sites without the
tag. The other two `Kind::Custom(7)` constructions in the tree
(`buzz-db/src/event.rs`, `desktop/src-tauri/src/commands/social.rs`) are
`#[cfg(test)]` fixtures.

Signed-off-by: Shani Singh <teamdeveloperworld@gmail.com>
@shani-singh1

Copy link
Copy Markdown
Author

Thanks — and the negative wasn't clean, so good question to ask. Swept the tree for kind:7 construction across all four languages and found one more emit site the original push missed: the Flutter client.

ChannelActions.addReaction (mobile/lib/features/channels/channel_management_provider.dart) emitted ["e", target] plus an optional NIP-30 emoji tag and no p tag — the same bug, just not reachable from any Rust call site, so the SDK signature change didn't surface it. Fixed in 8bc1e97: it now takes the target author, and all four call sites already had it (TimelineMessage.pubkey for the channel/thread rows and the action sheet, UserNote.pubkey threaded through toggleNoteUpvote for Pulse).

One asymmetry worth noting: mobile needs no self-tagging opt-out. The Dart signer (signed_event_relay.dart) passes tags through verbatim, so a self-reaction keeps its p tag without the .allow_self_tagging() the nostr 0.44 builders require.

Full sweep, for the record:

Site Status
buzz-sdk build_reaction / build_custom_emoji_reaction p tag ✅
desktop/src-tauri/src/events.rs::build_reaction p tag ✅
buzz-acp reaction_add (👀/💬) p tag ✅
buzz-cli cmd_add_reaction p tag ✅ (author via ids:[…] lookup)
mobile ChannelActions.addReaction p tag ✅ — added in 8bc1e97
buzz-db/src/event.rs:1827 #[cfg(test)] fixture, not an emit site
desktop/src-tauri/src/commands/social.rs:417 #[cfg(test)] fixture, not an emit site

Two deliberate exclusions:

Caveat on the mobile commit, stated plainly: there's no Dart toolchain on this machine (hermit's bootstrap fails on Windows), so I could not run dart analyze or flutter test on it. The change is mechanical — one signature, one tag, four call sites, all passing a non-nullable String field that already exists on the relevant model — and no mobile test references addReaction or toggleNoteUpvote. But CI is the first thing to actually compile it, so that's where to look if it's unhappy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Reactions without p-tags

2 participants